-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: search suggestion api #48
Conversation
The latest updates on your projects. Preview: https://vitamin-c-176e3fssn-poiu694s-projects.vercel.app
|
The latest updates on your projects. Preview: https://vitamin-c-fues29y2h-poiu694s-projects.vercel.app
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/utils/storage/session-storage.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: SessionStorageManager와 LocalStorageManager 모두가 extend 할 수 있는 storage.ts
를 만들어서 관리하는 것도 좋을 것 같네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
두 역할이 다른 것 같아서 빼긴 했는데! 합치는게 좋을까요?
두 인터페이스가 같아서 나중이나 리팩터링할 때 abstract class 를 storage.ts
에 넣는 느낌으로 생각하긴 했어용
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인터페이스가 같기도 하기도 하고, Window.sessionStorage와 Window.localStorage 모두 Storage object 인스턴스를 리턴하고 있으니까 비슷한 느낌으로 가려면 합치는게 좋을 것 같다고 생각했었어요! 생각해두신대로 나중에 리팩토링 가시죠~!
Cf. https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API
router.push(`${pathname}?${createQueryString('search', '')}`) | ||
} | ||
|
||
useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
react.dev에서 useLayoutEffect
사용을 지양하라고 해서, 언제 사용해야 성능 이슈보다 이점이 더 클지 잘 감이 안 오더라구요 useLayoutEffect
사용 기준을 따로 갖고 계신지 궁금합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useLayoutEffect는 레이아웃 관련 작업이나 깜박임 또는 레이아웃이 깨지는 문제를 방지할 때 말고 잘 안쓰긴 했는데, 여기선 Next라 useIsomorphicLayoutEffect로 넣긴 했네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
결국 useIsomorphicLayoutEffect
를 쓴다는 건 client side에서 useLayoutEffect
를 사용하겠다는 건데, 사용했을 때 깜빡임이 확실하게 사라지거나 더 빠르게 그려지는게 확실하지 않다보니, 성능 이슈를 고려해서 사용 기준을 명확하게 정해보는 것도 좋을 것 같아요!
The latest updates on your projects. Preview: https://vitamin-c-2ldfuieti-poiu694s-projects.vercel.app
|
Issue Number
#47
Description
Checklist
feat: add login page
)